Fix the following code review findings on aictrl-dev/cli PR #88 (head branch).
Run the relevant tests/linters after each change.

1. packages/cli/src/cli/cmd/run.ts:710 — Catalog built even when output is not JSON
   Detail: `emit()` only writes when `args.format === "json"`, but the catalog block unconditionally awaits `buildToolCatalogItems()` — which enumerates every MCP server via `listTools()` and scans the skill filesystem — even in the default human-readable mode where the result is never emitted. For interactive (non-JSON) sessions this adds unnecessary MCP round-trips and startup latency before the first turn.

Guard the collection on `args.format === "json"` (matching how the result is consumed), e.g. `if (args.format === "json") { await buildToolCatalogItems().then(...).catch(...) }`.
2. packages/cli/src/cli/cmd/run.ts:716-718 — Catalog errors swallowed with no logging
   Detail: The `.catch(() => {})` handler silently drops every error from `buildToolCatalogItems()`. The entire purpose of this event (per EVENTS.md) is to let the server-side gate detect "silent success" failure modes — but if catalog collection itself fails (e.g. `Skill.all()` throws on a malformed SKILL.md, `MCP.clients()` rejects, or `ToolRegistry.ids()` fails), the run completes with NO `tool_catalog` event at all and NO signal that anything went wrong. That is a brand-new silent-success path created by the very feature meant to eliminate them.

Because `buildToolCatalogItems` uses `Promise.all` over builtins, MCP, and skills, a single failing source (most likely `Skill.all()` hitting the filesystem) suppresses the whole catalog including the unrelated, healthy tool lists.

Please log the error (at least `log.warn`/`log.error`) inside the catch so operators can diagnose missing catalogs, while still keeping the session alive.
   Suggested fix:         .catch((err) => {
          // Never let catalog collection block or crash the session, but
          // surface the failure so a missing event is diagnosable.
          log.error("failed to emit tool_catalog", { error: err instanceof Error ? err.message : String(err) })
        })
3. packages/cli/src/cli/cmd/tool-catalog.ts:90 — Catalog builtins are unfiltered, not the dispatched set
   Detail: `buildToolCatalogItems()` resolves builtins via `ToolRegistry.ids()`, which returns the instance-level superset with NO model/agent filtering. But `resolveTools` (session/prompt.ts) dispatches via `ToolRegistry.tools({modelID, providerID}, agent, sessionID)`, which applies real per-model filters:

- `apply_patch` is only exposed for gpt-* models, and `edit`/`write` are HIDDEDEN for those same models — they are mutually exclusive at dispatch, yet the catalog will always report BOTH.
- `codesearch`/`websearch` are gated on `providerID === "aictrl" || AICTRL_ENABLE_EXA`, but the catalog always lists them.

EVENTS.md overstates the guarantee: it claims the gate "uses `tools[]` to verify that [tools] were actually in the model's function list at dispatch time." That is true for MCP tools (record_finding, etc.), but NOT for the builtin portion. A consumer that structurally checks for `edit` (or absence of `apply_patch`) on a gpt run will get wrong answers.

The author already notes "instance-level superset, without model-specific filtering" in a code comment — so the fix is either (a) pass model+agent into `buildToolCatalogItems` and call `ToolRegistry.tools(...)` so the catalog matches dispatch, or (b) correct EVENTS.md to scope the claim to MCP tools only.
4. packages/cli/src/mcp/index.ts:670-677 — listTools() runs twice per session start
   Detail: `MCP.toolEntries()` calls `client.listTools()` on every connected client at catalog time (before the first turn). Moments later, the first turn's `resolveTools` → `MCP.tools()` calls `client.listTools()` again on the same clients. For sessions with several MCP servers this doubles the round-trips to each server at session start, and for slow/remote MCP servers it adds latency directly on the critical path before the first model turn (the catalog `await` blocks `loop()`).

Consider memoizing the listTools result on the client/state for the session, or having `tools()` produce the entries so the work happens once.
5. packages/cli/src/mcp/index.ts:680-687 — MCP key derivation duplicated in toolEntries() and tools()
   Detail: `MCP.toolEntries()` re-implements the connected-client iteration and `sanitizedClientName + "_" + sanitizedToolName` key derivation that already lives in `MCP.tools()`. The catalog's value depends entirely on those keys matching what `resolveTools` exposes to the model (which comes from `MCP.tools()`'s keys). The two implementations must stay byte-for-byte in sync; if either sanitization changes, the catalog silently stops matching dispatch and the consumer gate produces false negatives/positives.

Extract a single shared helper (e.g. `mcpToolKey(clientName, toolName)` plus a shared `connectedClients()` iterator) so there is one source of truth, or have `tools()` delegate to `toolEntries()` for keying.

Fix the following code review findings on aictrl-dev/cli PR #88 (head branch).
Run the relevant tests/linters after each change.

1. EVENTS.md:17 — tool_catalog section placed before session_start
   Detail: The `tool_catalog` section is placed before `session_start` in EVENTS.md, but the event is emitted *after* `session_start` (the PR title and run.ts confirm this ordering). Documenting events in emission order makes the lifecycle narrative easier to follow. Consider moving the `tool_catalog` section to after `session_start`.
2. packages/cli/src/cli/cmd/run.ts:711 — Awaited catalog collection can block session start
   Detail: `buildToolCatalogItems()` is `await`ed before `loop()` starts. Inside, `MCP.toolEntries()` wraps each `client.listTools()` in `.catch()` for rejections, but a client that *hangs* (never resolves or rejects) will leave `Promise.all` pending indefinitely, stalling the entire session before the first turn. The `tools()` path has the same `listTools()` call but runs inside the agent loop where transport-level timeouts or loop guards may apply. Consider wrapping `toolEntries()` (or the `buildToolCatalogItems()` call in run.ts) with a bounded timeout so catalog collection cannot indefinitely block session start.
3. packages/cli/src/cli/cmd/run.ts:718 — Catalog failure logged to stderr, not stdout event stream
   Detail: When `buildToolCatalogItems()` rejects, the error is written to `console.error` (stderr). JSON consumers parse the NDJSON event stream on stdout and will not see this. The event's stated purpose is to detect "silent success" failure modes, but a catalog collection failure itself produces no structured signal on stdout — just a missing `tool_catalog` event plus an unstructured stderr line. Consider emitting a structured error/warning event (e.g. `tool_catalog` with an `error` field, or a dedicated `tool_catalog_error` event) so consumers reading stdout can distinguish "catalog failed" from "catalog succeeded with no MCP tools".
4. packages/cli/src/mcp/index.ts:668 — Duplicate listTools() calls double MCP round-trips
   Detail: `MCP.toolEntries()` calls `client.listTools()` on every connected client to build the catalog, and `MCP.tools()` later calls `client.listTools()` again on the same clients during `resolveTools` inside the agent loop. For JSON runs this means two full rounds of `listTools()` per MCP server before the first model turn. Consider memoising the `listTools()` result across both call sites, or having `toolEntries()` share state with the dispatch path.
5. packages/cli/src/skill/skill.ts:34 — version/metadata schema fields are type-only
   Detail: The `Info` schema adds `version: z.string().optional()` and `metadata: z.record(...)`, but the actual parse call uses `Info.pick({ name: true, description: true }).safeParse(md.data)`, so these fields are never validated by Zod — they exist only for the TypeScript type. The `version` field is particularly misleading: a contributor reading the schema might assume `version: 1` (YAML number) would be rejected, but it is silently accepted because `pick()` excludes it from validation. Consider adding a comment noting these are type-only, or validating them via a separate pick/parse if runtime validation is intended.